Skip to content

fix(ogc): raise on an unreadable date bound instead of dropping the filter - #441

Merged
thodson-usgs merged 1 commit into
DOI-USGS:mainfrom
thodson-usgs:fix/raise-on-unparseable-date-bound
Oct 8, 2026
Merged

thodson-usgs merged 1 commit into
DOI-USGS:mainfrom
thodson-usgs:fix/raise-on-unparseable-date-bound

Conversation

@thodson-usgs

@thodson-usgs thodson-usgs commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #440. _format_api_dates returned None when any non-blank element of a date argument matched no supported format, and both callers read None as "no date filter":

  • waterdata.get_ratings() sent the STAC search with no datetime and silently returned every rating.
  • The OGC getters (Water Data and NGWMN) sent an empty time=, and the service answered HTTP 400 "Invalid datetime format", naming no argument.

#440 fixed one input that took this path (".."). Any other unreadable value ("2024-13-45", "Jan 1 2024", ["2024-01-01", "tomorrow"]) still did.

Changes

  • ogc/dates.py: _format_one raises ValueError for a non-blank element it can't parse. The message names the caller's argument (name=) and the value, and gives accepted forms, including None for an open end. A range whose ends are all open still returns None (no filter). Returns/Raises/Notes updated.
  • Getter docstrings: the 15 time/begin/end/last_modified example blocks, plus get_ratings time and ngwmn.get_water_level datetime, now show the two-value list form with None for an open end. Until now that form was documented only in the private helper.
  • NEWS entry marked Behavior change. Only inputs that already failed are affected: for OGC getters, an HTTP 400 becomes a local ValueError. For get_ratings, a silently unfiltered result becomes an error.

Testing

  • New: test_format_api_dates_rejects_an_unreadable_bound (single, unsupported format, "Apr", range start/end), test_get_daily_rejects_an_unreadable_time_before_any_request, and test_get_ratings_rejects_an_unreadable_time_before_any_request. The two getter tests also assert that no request is sent.
  • The "Apr" case in test_format_api_dates pinned the old None. I moved it to the rejection test, where it still checks that "Apr" isn't mistaken for a duration.
  • Offline suite: 1254 passed under both TZ=UTC and TZ=America/Chicago. Branch coverage is 99.02%.
  • ruff check, ruff format --check, mypy, lint-imports, xenon, complexipy: pass.
  • Checked by hand: get_continuous(time="2024-13-45"), get_daily(last_modified=["bad", None]), and ngwmn.get_water_level(datetime=[..., "nope"]) each raise with the argument named.

Not in scope (already true on main)

  • A single string containing / is sent unchanged, so time="garbage/.." still reaches the service unvalidated. Validating each side would need to allow ISO 8601 start/duration intervals.
  • Non-string elements (pd.Timestamp, datetime) fail with AttributeError/TypeError instead of a ValueError that names the argument.

…ilter

_format_api_dates returned None when any non-blank element matched no
supported format. Callers treat None as "no date filter": get_ratings sent
its search without datetime and returned every rating, and the OGC getters
sent an empty time= that the service rejected with an HTTP 400 naming no
argument. DOI-USGS#440 fixed one input that hit this path (".."); every other
unreadable value still did.

The helper now raises ValueError naming the caller's argument and the bad
value, with the accepted forms, before any request. This covers time, begin,
end, datetime, last_modified, and the deprecated begin_utc/end_utc. A range
whose ends are all open still means no filter. The getter docstrings now show
the list form with None for an open end, which the new message points to.
@thodson-usgs
thodson-usgs force-pushed the fix/raise-on-unparseable-date-bound branch from 17708b5 to cc5c69b Compare October 8, 2026 14:31
@thodson-usgs
thodson-usgs marked this pull request as ready for review October 8, 2026 14:31
@thodson-usgs
thodson-usgs merged commit b3675aa into DOI-USGS:main Oct 8, 2026
11 checks passed
@thodson-usgs
thodson-usgs deleted the fix/raise-on-unparseable-date-bound branch October 8, 2026 14:34
thodson-usgs added a commit that referenced this pull request Oct 12, 2026
…ts (#442)

* feat(ogc): accept date, datetime and Timestamp values in date arguments

A datetime.date, datetime.datetime or pandas.Timestamp passed to a date
argument (time=[df.index.min(), None]) failed inside the formatter with
AttributeError ('endswith') or, for a lone datetime, TypeError (not
iterable), naming no argument. Read them like the equivalent string:
aware values convert to UTC, naive ones use the local zone as naive
strings do, a date is midnight of that day, and NaT is an open bound.
Any other type raises ValueError naming the argument.

Follow-up named in #441.

* fix(ratings): accept a lone date or Timestamp in get_ratings(time=...)

_validate_time_no_duration iterated a lone Timestamp/date and raised
TypeError before the formatter could read it; it now normalizes with
_coerce_to_list and checks only string elements.

Also:
- drop the redundant date check in _coerce_to_list (a date is not
  iterable), and update the Mapping TypeError to name the accepted types;
- read a bytes value as one element, so it gets the message naming the
  accepted types rather than "takes at most 2 values" over its byte codes;
- get_ratings' time docstring names the accepted date types;
- tighten the NEWS entry and date it by its land, above #446's.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HBt3Ee7eneKtdKQWq2T3YK

---------

Co-authored-by: thodson-usgs <thodson@usgs.gov>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
thodson-usgs added a commit that referenced this pull request Oct 12, 2026
…st form (#443)

* fix(ogc): read each side of a "start/end" interval string like the list form

_format_api_dates sent any single string containing "/" unchanged. The
same range spelled as the documented string ("2024-01-01T10:00:00/..")
and as the list (["2024-01-01T10:00:00", None]) could select different
data: the string skipped the local-to-UTC conversion of naive times, the
offset-to-Z conversion, and the date truncation for date-only
collections, and "not-a-date/also-bad" reached the service unvalidated.

Split the string on its "/" and format each side as a list element is
formatted, so an unreadable side raises ValueError naming the caller's
argument before any request. An empty side becomes "..", and "../.."
means no filter, as [None, None] does. One side may still be an ISO 8601
duration paired with an instant ("2024-01-01/P7D"), kept unchanged; any
other shape (three sides, two durations, a duration with an open end)
raises ValueError naming the argument. A lone duration ("P7D") is
unchanged.

Follow-up named in #441.

* fix(ogc): reject duration sides, open starts per API, naive dates as UTC

- Reject a duration as either side of an interval string. The Water Data
  and NGWMN services reject '2024-01-01/P7D' and 'P7D/2024-01-08' with
  HTTP 400, and that error does not name the argument. _split_interval
  raises instead and passes the two sides to the list path as a tuple,
  which removes the duration handling in _format_interval and the
  _is_passthrough helper.
- Write an open range start in the form each API accepts. The NGWMN and
  STAC firewalls reject any datetime containing '../' with HTTP 403, so
  get_water_level(datetime=[None, end]) and get_ratings(time=[None, end])
  always failed. The Water Data API accepts '../end' and rejects '/end'
  with HTTP 400. OgcDialect.open_start holds the form (default '..');
  NGWMN sets '', and get_ratings passes '' to _format_api_dates because
  STAC has no dialect.
- Keep fractional seconds instead of truncating them, which changed a
  closed bound. The Water Data, NGWMN, and STAC services all accept them
  (checked 2026-10-12). Accept a minute-precision time and spaces around
  the '/'; both were previously sent unchanged and worked.
- Read a naive date or time as UTC, as pandas does and as
  get_nearest_continuous already did, instead of as the machine's local
  time. Convert an aware value to UTC, including in a date-only
  collection. Now that the string form is parsed like the list form,
  local time would have made time="2024-03-01/2024-03-02", the example
  in the docs, start at 06:00Z in Chicago and on the previous day in
  Sydney.
- Docstrings: get_ratings no longer says time is passed through
  unchanged, and the _format_api_dates notes describe open_start. The
  duration test ids name the side that is a duration.
- Date the NEWS entry by the day it lands and place it above #446's.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HBt3Ee7eneKtdKQWq2T3YK

---------

Co-authored-by: thodson-usgs <thodson@usgs.gov>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant